Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make tree_data static functions available in api #2101

Merged
merged 1 commit into from
Sep 14, 2023
Merged

Make tree_data static functions available in api #2101

merged 1 commit into from
Sep 14, 2023

Conversation

irfanHaslanded
Copy link
Contributor

We need certain functions to be exposed via a public API to make it
possible to use in other languages such as Rust.

lyd_parent, lyd_child, and lyd_get_value are defined as static inline
and is difficult for tools to wrap them.

michalvasko
michalvasko reviewed Sep 14, 2023
View reviewed changes
Copy link
Member

@michalvasko michalvasko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I am fine with this but I am not sure making the functions inline has any benefit, I would think the compilers are smart enough to do it themselves as needed. So you may simply change them into normal functions.

@irfanHaslanded
Copy link
Contributor Author

Thanks Michal, I will do that then.

michalvasko
michalvasko requested changes Sep 14, 2023
View reviewed changes
src/tree_data.h Outdated Show resolved Hide resolved
Make tree_data static functions available in api
89dde2f 
 We need certain functions to be exposed via a public API to make it
 possible to use in other languages such as Rust.

 lyd_parent, lyd_child, and lyd_get_value are defined as static inline
 and is difficult for tools to wrap them.
@jktjkt
Copy link
Contributor

jktjkt commented Sep 14, 2023

lyd_parent, lyd_child, and lyd_get_value are defined as static inline and is difficult for tools to wrap them.

How come? These functions are defined in a public header, so they should be usable just fine, AFAICT. What is the exact problem that you're solving with this patch, and what error message in which scenario are you getting without this patch?

@michalvasko michalvasko merged commit d3b351a into CESNET:devel Sep 14, 2023
12 checks passed
@irfanHaslanded irfanHaslanded deleted the irfan/gnmi-needed-api branch September 14, 2023 14:33
@irfanHaslanded
Copy link
Contributor Author

lyd_parent, lyd_child, and lyd_get_value are defined as static inline and is difficult for tools to wrap them.

How come? These functions are defined in a public header, so they should be usable just fine, AFAICT. What is the exact problem that you're solving with this patch, and what error message in which scenario are you getting without this patch?

If we have to use these functions in another language such as Rust, tools such as rust-bindgen, do not easily generate wrappers for them.

@jktjkt
Copy link
Contributor

jktjkt commented Sep 15, 2023

That's been supported since 2022, rust-lang/rust-bindgen#2335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalvasko michalvasko michalvasko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@irfanHaslanded @jktjkt @michalvasko